Skip to content

gh-141510, PEP 814: Add built-in frozendict type#144757

Open
vstinner wants to merge 13 commits intopython:mainfrom
vstinner:frozendict_base
Open

gh-141510, PEP 814: Add built-in frozendict type#144757
vstinner wants to merge 13 commits intopython:mainfrom
vstinner:frozendict_base

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Feb 12, 2026

Add TYPE_FROZENDICT to the marshal module.

Add C API functions:

  • PyAnyDict_Check()
  • PyAnyDict_CheckExact()
  • PyFrozenDict_Check()
  • PyFrozenDict_CheckExact()
  • PyFrozenDict_New()

Add PyFrozenDict_Type C type.


📚 Documentation preview 📚: https://cpython-previews--144757.org.readthedocs.build/en/144757/c-api/dict.html#frozen-dictionary-objects

Add TYPE_FROZENDICT to the marshal module.

Add C API functions:

* PyAnyDict_Check()
* PyAnyDict_CheckExact()
* PyFrozenDict_Check()
* PyFrozenDict_CheckExact()
* PyFrozenDict_New()

Add PyFrozenDict_Type C type.
@vstinner
Copy link
Member Author

cc @corona10

Fix also indentation.
@corona10 corona10 self-requested a review February 13, 2026 10:07
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
}

if (PyDict_CheckExact(d)) {
if (PyAnyDict_CheckExact(d)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe same opinion with @kumaraditya303 we can avoid a lot of critical section if dict is frozen dict.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but I would prefer to make such "optimization" change in a separated PR.

dict_length(PyObject *self)
{
return FT_ATOMIC_LOAD_SSIZE_RELAXED(((PyDictObject *)self)->ma_used);
return GET_USED(_PyAnyDict_CAST(self));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to use atomic operation for frozendict?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we can skip the atomic operation for frozendict.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It uses relaxed atomic so it compiles to simple loads so avoiding it has no real benefit.

};

static PyMappingMethods frozendict_as_mapping = {
.mp_length = dict_length,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we can make frozendict_lenth

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to make such optimization in a separated PR.


static PyMappingMethods frozendict_as_mapping = {
.mp_length = dict_length,
.mp_subscript = dict_subscript,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still perfer to make frozendict_subscript, but let's do it at the separate PR.

Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should notice that frozendict is not subclass of dict to follow SC feedback

vstinner and others added 3 commits February 13, 2026 15:46
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Document frozendict after lazy import.
mp->ma_values == NULL &&
(mp->ma_used >= (mp->ma_keys->dk_nentries * 2) / 3))
(mp->ma_used >= (mp->ma_keys->dk_nentries * 2) / 3) &&
!frozendict)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might take this fast-path for frozendict as well. It's just a missed optimization opportunity.

@corona10: Add it to the optimization TODO list :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I will take a look at it

if (items == NULL) {
return -1;
}
PyObject *frozenset = PyFrozenSet_New(items);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current implementation creates an actual frozenset object which emits surprising error messages:

>>> hash(frozendict(x=[]))
Traceback (most recent call last):
  File "<python-input-0>", line 1, in <module>
    hash(frozendict(x=[]))
    ~~~~^^^^^^^^^^^^^^^^^^
TypeError: cannot use 'tuple' as a set element (unhashable type: 'list')

We may change the implementation later to not create an actual frozenset, but reuse frozenset hash code instead, to avoid the surprising errors.

@vstinner
Copy link
Member Author

We should notice that frozendict is not subclass of dict to follow SC feedback

Ah right, done.

@corona10 corona10 added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Feb 14, 2026
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @corona10 for commit 9101b1a 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F144757%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Feb 14, 2026
@corona10
Copy link
Member

Let's wait build bot to pass the refleak test :)

Copy link
Contributor

@benediktjohannes benediktjohannes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: I know this does not primarily belong to the PR, but it's such a small change that a new PR wouldn't make sense for these small "typos".

Co-authored-by: Adam Johnson <me@adamj.eu>
@vstinner
Copy link
Member Author

Let's wait build bot to pass the refleak test :)

Tests passed successfully on Refleaks buildbots:

buildbot/AMD64 CentOS9 NoGIL Refleaks PR — Build done.
buildbot/AMD64 Fedora Stable Refleaks PR — Build done.
buildbot/AMD64 FreeBSD Refleaks PR — Build done.
buildbot/AMD64 RHEL8 Refleaks PR — Build done.
buildbot/AMD64 Windows11 Refleaks PR — Build done.
buildbot/ARM64 MacOS M1 Refleaks NoGIL PR — Build done.
buildbot/PPC64LE Fedora Stable Refleaks PR — Build done.
buildbot/PPC64LE RHEL8 Refleaks PR — Build done.
buildbot/s390x Fedora Stable Refleaks PR — Build done.
buildbot/s390x RHEL8 Refleaks PR — Build done.
buildbot/s390x RHEL9 Refleaks PR — Build done.

vstinner and others added 2 commits February 16, 2026 13:01
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Add also a title in Doc/library/stdtypes.rst.
@vstinner
Copy link
Member Author

@hugovk: I modified a few more titles in the C API doc and the Python doc to use sentence case for headers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants